Skip to content

Conversation

pranavshenoy
Copy link
Contributor

@pranavshenoy pranavshenoy commented Sep 17, 2025

add SSTableFlushObserver#onSSTableWriterSwitched to flush SAI segment builder for written shards without waiting for entire transaction to complete

This reduces memory usage during sharded compaction.

The Cassandra Jira

@pranavshenoy pranavshenoy force-pushed the sstableindex_flushing branch 2 times, most recently from 9981933 to c446387 Compare September 19, 2025 04:57
boolean emptySegment = currentBuilder == null || currentBuilder.isEmpty();
logger.debug("Flushing index {} with {}buffered data on sstable writer switched...", index.identifier().indexName, emptySegment ? "no " : "");
if (!emptySegment)
flushSegment();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that we don't have to aggressively create a new builder here because addTerm(), complete(), and abort() will all handle the currentBuilder == null case appropriately.

Copy link
Contributor

@maedhroz maedhroz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropped some comments throughout the PR. We should probably also make sure we don't refer to CNDB-14725 in the commit message ;)

@pranavshenoy pranavshenoy force-pushed the sstableindex_flushing branch 2 times, most recently from 5f2deb4 to 55d7132 Compare October 7, 2025 05:16
… SAI segment builder for written shards without waiting for entire transaction to complete (apache#1859)

This reduces memory usage during sharded compaction.

riptano/cndb#14725 OOM during SAI compaction
with large num of shards

Flush segment builder when sstable writer is switched to free memory
without waiting full compaction to complete
@pranavshenoy pranavshenoy force-pushed the sstableindex_flushing branch from 55d7132 to 27dbb77 Compare October 7, 2025 05:26
@pranavshenoy pranavshenoy requested a review from maedhroz October 7, 2025 05:33
Copy link
Contributor

@jasonstack jasonstack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants